@@            Coverage Diff             @@
##             main    #3667      +/-   ##
==========================================
+ Coverage   54.30%   54.43%   +0.13%     
==========================================
  Files        2349     2354       +5     
  Lines       89718    89802      +84     
  Branches    25119    25069      -50     
==========================================
+ Hits        48722    48885     +163     
- Misses      40648    40656       +8     
+ Partials      348      261      -87     

*This pull request uses carry forward flags. Click here to find out more.

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 00f80fc...02ee320. Read the comment docs.

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Review

Findings

Medium

• [logic-error] workspaces/scorecard/plugins/scorecard/src/components/MetricGroupCard/DataSourcesDialog.tsx:601 — The handleOverlayClick callback uses document.querySelector('[role="dialog"]') to find the dialog element, which returns the first element in DOM order matching that selector. If any other dialog is present in the DOM (e.g., from Backstage or another plugin), this check may match the wrong element, causing clicks inside the real DataSourcesDialog to be misidentified as outside clicks (closing the dialog unexpectedly) or clicks outside to be ignored.
  Remediation: Use a React ref attached to the Dialog's content container and check ref.current.contains(e.target) instead of querying the DOM by role.

Low

• [edge-case] workspaces/scorecard/plugins/scorecard/src/components/MetricGroupCard/thresholdBucketUtils.ts:49 — collectThresholdRules uses a first-seen-wins strategy: when multiple metrics have threshold rules with the same key but different expressions, only the first metric's expression is kept. The resulting ThresholdBucket.expression shown in the legend may be misleading when a group contains metrics with heterogeneous threshold definitions.

• [pattern-violation] workspaces/scorecard/plugins/scorecard/src/components/MetricGroupCard/MetricGroupCard.tsx:42 — buildThresholdBuckets(metrics, t) is called directly in the render body without useMemo, so it re-runs on every render. The same function call in DataSourcesDialog.tsx is correctly wrapped in useMemo. While not a functional bug, this is inconsistent and causes unnecessary recomputation.

• [edge-case] workspaces/scorecard/plugins/scorecard/src/components/MetricGroupCard/DataSourcesDialog.tsx:580 — The sort comparator for the 'value' column treats non-numeric values (like the em-dash '—') as equal to each other (return 0), meaning their relative order is unstable across sort toggles. This is a minor UX inconsistency rather than a crash.

Review

Findings

Medium

• [logic-error] workspaces/scorecard/plugins/scorecard/src/components/MetricGroupCard/DataSourcesDialog.tsx — The handleOverlayClick handler uses document.querySelector('[role="dialog"]') to detect clicks outside the dialog. This returns the first matching element in document order, which will malfunction if any other dialog/modal with role="dialog" is present in the DOM. Additionally, this manual dismiss logic is redundant with the Dialog component's own onOpenChange handler (already wired at line 559), creating a potential double-close race.
  Remediation: Remove the handleOverlayClick callback and its associated useEffect. Rely solely on the Dialog component's built-in onOpenChange prop for dismiss-on-overlay-click behavior.

• [logic-error] workspaces/scorecard/plugins/scorecard/src/components/MetricGroupCard/DataSourcesDialog.tsx — Global <style> tags with broad selectors ([class*="DialogOverlay"], [slot="title"], [slot="close"] svg) are injected whenever the dialog is open. These selectors match any element on the page with those attributes/class substrings, not just elements within this dialog instance. This can unintentionally restyle other Backstage UI Dialog instances or slot-based components rendered elsewhere in the application.
  Remediation: Scope these styles to a wrapper element using a unique CSS class or use the Dialog component's built-in styling props/slots instead of global <style> injection.

Low

• [missing-authorization] No linked issue — This PR introduces a substantial new feature (MetricGroupCard component family, DataSourcesDialog, Masonry layout, removal of scorecardEntityLayoutList, new @mui/lab dependency, i18n keys across 6 locales) but has no linked issue. The PR carries the do-not-merge/work-in-progress label, so this may be addressed before the PR is ready for merge.

• [breaking-api] workspaces/scorecard/plugins/scorecard/src/alpha/index.tsx — The scorecardEntityLayoutList extension is removed from scorecardCatalogModule.extensions. This extension was @alpha and disabled: true by default, limiting the blast radius to alpha consumers who explicitly opted in. Consider documenting this removal in the changeset.

• [scope-creep] workspaces/scorecard/plugins/scorecard/src/components/Scorecard/Scorecard.tsx — The Scorecard component's width is changed from 371px to 100%. While this is necessary for the new Masonry layout, it also affects the default EntityScorecardContent view. Documenting why this change is necessary would improve reviewability.

• [scope-creep] workspaces/scorecard/plugins/scorecard/src/components/Scorecard/EntityScorecardContent.tsx — Layout change from flex-wrap to CSS grid and donut chart cx change from 22% to 82px. These are related to the width change above but should be documented.

• [scope-creep] workspaces/scorecard/plugins/scorecard/src/alpha/extensions/scorecardLayoutExtensions.tsx — Removing scorecardEntityLayoutList is a natural cleanup alongside the ScorecardEntityContentGridView refactor, but the PR description should mention this removal.

• [component-declaration-style] workspaces/scorecard/plugins/scorecard/src/components/MetricGroupCard/StatusIcon.tsx — Declared as export function while the codebase convention for exported components is export const ComponentName = (...) =>.

• [component-declaration-style] workspaces/scorecard/plugins/scorecard/src/components/MetricGroupCard/ThresholdLegend.tsx — Same as above: uses export function instead of export const arrow.

• [type-definition-style] workspaces/scorecard/plugins/scorecard/src/components/MetricGroupCard/types.ts — MetricGroupCardProps uses export interface while all other exported types in types.ts files use export type. The same file uses export type for ThresholdBucket, creating an internal inconsistency.

• [edge-case] workspaces/scorecard/plugins/scorecard/src/components/MetricGroupCard/thresholdBucketUtils.ts — collectThresholdRules deduplicates rules by key, keeping only the first occurrence. If metrics within a group have different threshold definitions for the same key, the bucket's expression will reflect whichever metric appears first.

• [dependency-coherence] workspaces/scorecard/plugins/scorecard/package.json — @mui/lab@5.0.0-alpha.177 is added as a production dependency. Only used for the Masonry component. While @mui/lab is widely used in the MUI ecosystem despite its alpha versioning, confirm this is acceptable per project standards.

• [dependency-promotion] workspaces/scorecard/plugins/scorecard/package.json — @backstage/ui moved from devDependencies to dependencies (correct since new components import from it at runtime). @mui/lab is a new transitive dependency for consumers.